Skip to content

NO TICKET: add session_ctx to development version init#251

Merged
jirhiker merged 1 commit into
stagingfrom
cm-session-test
Nov 17, 2025
Merged

NO TICKET: add session_ctx to development version init#251
jirhiker merged 1 commit into
stagingfrom
cm-session-test

Conversation

@chasetmartin

@chasetmartin chasetmartin commented Nov 17, 2025

Copy link
Copy Markdown
Collaborator

Why

This PR addresses the following problem / context:

  • I was getting this error on docker compose up --build
app-1  |   File "/app/core/app.py", line 42, in lifespan                                          
app-1  |     erase_and_rebuild_db()                                                               
app-1  |     ~~~~~~~~~~~~~~~~~~~~^^                                                               
app-1  | TypeError: erase_and_rebuild_db() missing 1 required positional argument: 'session'

How

Implementation summary - the following was changed / added / removed:

  • @jirhiker I'm not confident this is the best way to handle this, but please let me know your thoughts. I believe this is causing all of the issues @TylerAdamMartinez was having with the CI_Cypress workflow on OcotilloUI. The docker compose up command fails without erase_and_rebuild_db() having the session argument. The OcotilloUI github actions uses docker compose up to build the app to test against.
  • I tried a draft PR from OcotilloUI against this branch and the CI_Cypress workflow worked.

Notes

Any special considerations, workarounds, or follow-up work to note?

  • Use bullet points here

@codecov-commenter

codecov-commenter commented Nov 17, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
core/app.py 33.33% 2 Missing ⚠️
Files with missing lines Coverage Δ
core/app.py 30.12% <33.33%> (ø)

... and 19 files with indirect coverage changes

jacob-a-brown
jacob-a-brown approved these changes Nov 17, 2025
View reviewed changes

@jacob-a-brown jacob-a-brown left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Another option is to invoke

with session_ctx():
    ....

in the function definition and then update erase_and_rebuild_db(session) to erase_and_rebuild_db() throughout the repo if you don't want to import/use the session in core/app.py

@jirhiker jirhiker merged commit 2d2ca8b into staging Nov 17, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jirhiker jirhiker jirhiker approved these changes
@jacob-a-brown jacob-a-brown jacob-a-brown approved these changes
@TylerAdamMartinez TylerAdamMartinez Awaiting requested review from TylerAdamMartinez

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@chasetmartin @codecov-commenter @jirhiker @jacob-a-brown